Skip to content

Require data-sources directory in addition to models directory in app.boot(). - #284

Closed
STRML wants to merge 1 commit into
strongloop:masterfrom
STRML:loadDataSourcesFolder
Closed

Require data-sources directory in addition to models directory in app.boot().#284
STRML wants to merge 1 commit into
strongloop:masterfrom
STRML:loadDataSourcesFolder

Conversation

@STRML

@STRML STRML commented May 27, 2014

Copy link
Copy Markdown
Member

This prevents a potential issue where a model is loaded from config JSON that specifies use a datasource
that is loaded via JS (e.g. via a module in ./data-sources). The model would typically be loaded before the datasource in this case, leading to an assertion error as the datasource is not properly set up.

This allows the files in data-sources to be loaded with the full appConfig attached to the app object, allowing a datasource module to use environment configuration in its configuration.

This prevents a potential issue where a model is loaded from config that specifies a datasource
that is loaded via JS. The model would typically be loaded before the datasource in this case,
leading to an error.

This allows the files in `data-sources` to be loaded with the full `appConfig` attached
to the app object.

Signed-off-by: Samuel Reed <sam@tixelated.com>
@slnode

slnode commented May 27, 2014

Copy link
Copy Markdown

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bajtos

bajtos commented May 28, 2014

Copy link
Copy Markdown
Member

Hey @STRML, thank you for making the effort to submit the pull request.

We are in process of moving app.boot to a standalone module that can be versioned independently of loopback core:

strongloop/loopback-boot#1

I have a different plan for dynamic definition of datasources. Instead of requiring all files in datasources directory, there loopback will load and merge configuration from files:

  • datasources.json
  • datasources.local.js or datasources.local.json
  • datasources.{$env}.js or datasoources.{$env}.json

where $env is the value of app.get('env'), which usually defaults to process.env.NODE_ENV. See strongloop/loopback-boot@c1743dc for more details.

I am very reluctant to allow developers to add datasources from other places than datasources.json, as it would it make difficult for tools like slc lb model (and the upcoming yeoman generator-loopback) to work with datasource configuration.

With the implementation proposed for loopback-boot, you would put all static configuration of your datasource to datasources.json. Then you can alter this static configuration in datasources.local.js.

@STRML

STRML commented May 28, 2014

Copy link
Copy Markdown
Member Author

That works for me and is better than this monkey patch. My main concern is per-environment configuration of datasources, which is not elegant in the current version. Thank you for the lengthy reply, I will watch loopback-boot.

@STRML STRML closed this May 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants